-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use sourcegen bytecode generator to generate internal metadata #11407
base: 4.8.x
Are you sure you want to change the base?
Conversation
inject/src/main/java/io/micronaut/inject/beans/AbstractInitializableBeanIntrospection.java
Outdated
Show resolved
Hide resolved
defaultsStorage, | ||
loadTypeMethods | ||
public static byte[] write(String className, AnnotationMetadata annotationMetadata) { | ||
Map<String, MethodDef> loadTypeMethods = new HashMap<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be a linked hash map? Will it impact the order of the methods written?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, it's only used for testing anyway
core-processor/src/main/java/io/micronaut/inject/annotation/AnnotationMetadataWriter.java
Outdated
Show resolved
Hide resolved
core-processor/src/main/java/io/micronaut/aop/writer/AopProxyWriter.java
Show resolved
Hide resolved
core-processor/src/main/java/io/micronaut/aop/writer/AopProxyWriter.java
Outdated
Show resolved
Hide resolved
core-processor/src/main/java/io/micronaut/aop/writer/AopProxyWriter.java
Outdated
Show resolved
Hide resolved
* @param p The class element | ||
* @return The string representation | ||
*/ | ||
private static String toTypeString(ClassElement p) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems like something that belongs in ClassElement
or a utility method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, we just do some unique string from a class
core-processor/src/main/java/io/micronaut/inject/writer/BeanDefinitionWriter.java
Outdated
Show resolved
Hide resolved
core-processor/src/main/java/io/micronaut/inject/writer/BeanDefinitionWriter.java
Show resolved
Hide resolved
Quality Gate failedFailed conditions See analysis details on SonarQube Cloud Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to make sure that we retain that the generated byte code is synthetic
proxyClass, | ||
proxyClass, | ||
"<init>", | ||
newConstructorParameters.toArray(ZERO_PARAMETER_ELEMENTS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be good to add a MethodElement.constructorOf
method that makes this simple
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very unique case, I don't see a point of adding a new method
@@ -373,18 +444,8 @@ public boolean isProxyTarget() { | |||
} | |||
|
|||
@Override | |||
protected void startClass(ClassVisitor classWriter, String className, Type superType) { | |||
String[] interfaces = getImplementedInterfaceInternalNames(); | |||
classWriter.visit(V17, ACC_SYNTHETIC, className, null, !isInterface ? superType.getInternalName() : null, interfaces); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sure that whatever we are generated retains ACC_SYNTHETIC
modifier. I can't seem to see where this is retained?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I need to add a support for it first
core-processor/src/main/java/io/micronaut/aop/writer/AopProxyWriter.java
Outdated
Show resolved
Hide resolved
core-processor/src/main/java/io/micronaut/inject/annotation/AnnotationMetadataGenUtils.java
Outdated
Show resolved
Hide resolved
core-processor/src/main/java/io/micronaut/inject/writer/ExpressionsUtils.java
Outdated
Show resolved
Hide resolved
* @since 4.7 | ||
*/ | ||
@Internal | ||
public final class MethodGenUtils { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a private
constructor
The PR rewrites all the class writers (except the evaluated expressions writer) to use a new bytecode generator from Micronaut Sourcegen.
I haven't done any major refactoring. Some classes like
BeanDefnitionWriter
is too big and need some refactoring